libexpr: deprecate the bogus "or"-as-variable #11560
Conversation
As a prelude to making "or" work like a normal variable, emit a warning any time the "fn or" production is used in a context that will change how it is parsed when that production is refactored. In detail: in the future, OR_KW will be moved to expr_simple, and the cursed ExprCall production that is currently part of the expr_select nonterminal will be generated "normally" in expr_app instead. Any productions that accept an expr_select will be affected, except for the expr_app nonterminal itself (because, while expr_app has a production accepting a bare expr_select, its other production will continue to accept "fn or" expressions). So all we need to do is emit an appropriate warning when an expr_simple representing a cursed ExprCall is accepted in one of those productions without first going through expr_app. As the warning message describes, users can suppress the warning by wrapping their problematic "fn or" expressions in parentheses. For example, "f g or" can be made future-proof by rewriting it as "f (g or)"; similarly "[ x y or ]" can be rewritten as "[ x (y or) ]", etc. The parentheses preserve the current grouping behavior, as in the future "f g or" will be parsed as "(f g) or", just like "f g anything-else" is grouped. (Mechanically, this suppresses the warning because the problem ExprCalls go through the "expr_app : expr_select" production, which resets the cursed status on the ExprCall.)
github-actions
bot
added
the
with-tests
|
Realistically, the special |
|
This is an implementation of #11121 (comment), not a prelude to removing support for |
tomberek
added
the
idea approved
|
So we discussed this in the team meeting. We just want to make sure our flake regression suite passes without too many warnings: https://github.com/NixOS/flake-regressions/ Otherwise we need @edolstra for code review. |
|
Are you asking me to do the flake regression test, or just updating on status? |
It was informative because I think Eelco was going to run it, but it helps if you can. Plus it helps to have more people familiar with it. |
|
Attempting to do so, it appears that the directory added in NixOS/flake-regressions-data@c73996a is not at the level of nesting expected by the eval-all.sh script. Could be user error though. |
|
Well, I've evaluated every flake in the regression suite, and while not every test passed, there are no occurrences of the new warning (as should be expected). |
|
Sounds good to me than. |
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/2024-09-30-nix-team-meeting-minutes-182/53814/1 |
Motivation
As a prelude to making
orwork like a normal variable, emit a warning any time thefn orproduction is used in a context that will change how it is parsed when that production is refactored.In detail: in the future,
OR_KWwill be moved toexpr_simple, and the cursed ExprCall production that is currently part of theexpr_selectnonterminal will be generated "normally" inexpr_appinstead. Any productions that accept anexpr_selectwill be affected, except for theexpr_appnonterminal itself (because, whileexpr_apphas a production accepting a bareexpr_select, its other production will continue to acceptfn orexpressions). So all we need to do is emit an appropriate warning when anexpr_simplerepresenting a cursed ExprCall is accepted in one of those productions without first going throughexpr_app.As the warning message describes, users can suppress the warning by wrapping their problematic
fn orexpressions in parentheses. For example,f g orcan be made future-proof by rewriting it asf (g or); similarly[ x y or ]can be rewritten as[ x (y or) ], etc. The parentheses preserve the current grouping behavior, as in the futuref g orwill be parsed as(f g) or, just likef g anything-elseis grouped. (Mechanically, this suppresses the warning because the problem ExprCalls go through theexpr_app : expr_selectproduction, which resets the cursed status on the ExprCall.)Context
Forked from #11121, which will be rebased atop this once it is merged and then sit around for however many years it's decided is an appropriate deprecation period.
Priorities and Process
Add 👍 to pull requests you find important.
The Nix maintainer team uses a GitHub project board to schedule and track reviews.